Repository navigation
Conversation
…r answer again The cache key carries the five-minute window, so every window is a new key and nothing ever removed an old one: a board that polled kept 288 dead buckets per day for each repository and zone, for the life of the process (#2806). A write now sweeps the entries whose expiresAt has passed, and CACHE_MAX bounds a table whose writes outpace its reads because nothing sweeps in between. A hit inside the same window, METRICS_TTL_MS and the shape of the answer are unchanged, and a caller-supplied store that answers only get/set is left as it was. Tests: core/test/project-metrics-cache-bound.test.js — red before the fix (bucket 0 answered `hit` after 200 later windows; 201 entries retained) and green after it, with the two existing project-metrics suites green. Fixes #2806 Docs-not-needed: core-host.md's project-metrics paragraph still describes the same key, the same five-minute lifetime, the same hit and the same answer; this change only bounds how long an entry that can never be answered is retained. Written-by: Dev
Collaborator
Author
|
The body's Written-by: Dev |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix(project-metrics): drop the five-minute buckets the cache can never answer again
Fixes #2806
Owner: Dev
Reviewer: Reviewer
Written-by: Dev
Bot: Dev
Risk tier: T2
Base: f42ad01
Claim-Id: 2bf659a9a3584cd2ae0ca7050630a0a2
What changed
core/src/host/project-metrics.jskept its shared cache for the life of the process. The key isrepo|zone|<now / METRICS_TTL_MS>, so every five minutes a new key arrived and the entry under theold one was never removed — the read at
:196looked atexpiresAtonly to decide the hit, and theentry stayed. A board that polls its metrics therefore added 288 dead buckets a day for each
repository and zone, each one still holding a full metric value (issue #2806: 12,001 buckets, 8.33 MB
retained, and bucket 0 still answering
hittwelve thousand windows later).pruneCache(store, now)runs before the write: it deletes every entry whoseexpiresAt <= now,which is exactly the set no later read could answer. The sweep is on the write because that is the
only moment this table changes.
CACHE_MAX = 512bounds a table whose writes outpace its reads (nothing sweeps in between): theoldest entries are dropped in insertion order.
METRICS_TTL_MS, a hit inside the same window, and the answer'sshape.
project-board.jsis not touched, and a caller-injected store that answers onlyget/setis left as it was rather than given a
deleteit never promised.core/test/project-metrics-cache-bound.test.jsis new: an explicitMap, bucket 0 read, 200 laterbuckets, then bucket 0 again. Before the fix that second read answered
hitand the map held 201entries; now it is a
missand the map holds at most 2.patches/ds-bot/files/**is whatnode scripts/ds-bot-patches.mjs generatewrote for the twocore/changes — nothing hand-edited.Ran
At the tip
6e8815fc6f01322e0f484ff5a193873f8ef01611:node --test core/test/project-metrics-cache-bound.test.js— red before the fix(
'hit' !== 'miss'), green after it.node --import ./test/harness-resolve.mjs --test test/project-metrics.test.js test/project-metrics-scope.test.js test/project-metrics-cache-bound.test.jsincore/— 26 pass.node core/scripts/check-arch-docs.mjs— OK (5 changed files), the commit and this body both carrythe
Docs-not-needed:line.node core/scripts/build-client.mjs --check,node scripts/ds-bot-patches.mjs check,node scripts/ds-bot-patches.mjs canonical— all green.node .github/scripts/changelog.mjs check— green.Not run: the smoke test and the DSH bench (no change to
index.jsor a client bundle).TZ=UTC npm run verifyin this working copy reaches the core suite and reports 3195/3198 with twofailures —
bench-preflight"bench.sh refuses before writing the profile when pnpm is missing" andcheck-arch-docs"the gate never runs git through a shell". Both also fail on unmodifiedmainin thesame checkout (checked with the change stashed), and neither test reads
project-metrics.js; they areworking-copy artifacts (the fixture tree sits inside a git worktree, and
/tmpis not executable), soCI's
tests-okis the authority for them.Docs-not-needed:
core-host.md's project-metrics paragraph still describes the same key, the samefive-minute lifetime, the same hit and the same answer; this change only bounds how long an entry that
can never be answered is retained.
{
"pr": 2810,
"claim": "Dev",
"issue": 2806,
"summary": "The metrics cache keyed by the five-minute window never dropped a bucket it could no longer answer, so a polling board kept 288 dead entries a day per repository and zone for the life of the host. A write now sweeps entries whose expiresAt has passed and CACHE_MAX bounds a table whose writes outpace its reads. A hit inside the same window, the TTL and the answer's shape are unchanged, and project-board.js is not touched. The new test reads bucket 0, fills 200 later buckets and reads bucket 0 again: it answered hit before the fix and is a miss now, with the map bounded.",
"files": [
"core/src/host/project-metrics.js",
"core/test/project-metrics-cache-bound.test.js",
"changelog.d/2806.fixed.md",
"patches/ds-bot/files/src/host/project-metrics.js",
"patches/ds-bot/files/test/project-metrics-cache-bound.test.js"
],
"checks": {
"ran": "node --test core/test/project-metrics-cache-bound.test.js; npm --prefix core test targeted at project-metrics*.test.js; node core/scripts/check-arch-docs.mjs; node core/scripts/build-client.mjs --check; node scripts/ds-bot-patches.mjs check; node scripts/ds-bot-patches.mjs canonical; node .github/scripts/changelog.mjs check",
"result": "green",
"at": "6e8815fc6f01322e0f484ff5a193873f8ef01611"
},
"next": "none — ready for review"
}
Written-by: Dev